[WIP] Be able to stop reconnection loop#35
[WIP] Be able to stop reconnection loop#35craftytrickster merged 6 commits intocraftytrickster:mainfrom
Conversation
craftytrickster
left a comment
craftytrickster
left a comment
There was a problem hiding this comment.
Thank you for taking a look at this. If my theory about the lack of ability to interrupt a long lasting Duration is true, some changes will need to be made.
The way I see it, there are at least two approaches we can take:
-
Have some kind of cloneable token/signal that can be passed into the reconnection options. The user can store the token wherever they want, and when they invoke it, it will cause the reconnect poll to immediately exit without waiting any further. We can make our own struct that is simply a wrapper around a channel sender.
-
Change the API on the options from
DurationIteratortoDurationStreamofStream<Item=()>, where each item is separated by a waiting duration. Because the stream is user definable, it would allow the user to create their own stream very easily using a tokio::sync::mpsc::channel. In most cases they can treat the stream exactly as the iterator is working now, however, they could also asynchronously inject aPoll::Ready(None)to close the stream whenever they want.
I personally would go with approach 1, because we can do it without it being a breaking API change, and 2, because wanting to interrupt/halt an attempt reconnect at application shutdown seems like a standard enough case that it would make sense to do so.
What are your thoughts?
src/tokio/io.rs
Outdated
| cx.waker().wake_by_ref(); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
We need to add a test where the reconnection Durations (from the retries_to_attempt_fn) are extremely long, and then the user calls stop_reconnection.
In this case, if the connection was in a disconnected state, and the user calls stop_reconnection, I would expect the connection to immediately stop waiting for the duration to complete, and to just exit. I think with the code you implemented here, it will not stop waiting, it will still wait for the entire length of the currently active Duration provided by the iterator, which might be an unreasonably long amount of time.
src/tokio/io.rs
Outdated
| } | ||
|
|
||
| /// Stops any further reconnect attempts. | ||
| pub fn stop_reconnection(&mut self) { |
There was a problem hiding this comment.
I would not want to force the user to have to do this by mutable self ref, perhaps an atomic bool would make more sense.
First approach, WIP |
akuskis
changed the title
craftytrickster
left a comment
craftytrickster
left a comment
There was a problem hiding this comment.
This seems like the right approach, still needs testing
akuskis
force-pushed
the
introduce_shutdown_flag
branch
from
c51fb21 to
0d4c455
Compare
craftytrickster
left a comment
craftytrickster
left a comment
There was a problem hiding this comment.
The implementation looks good, a little more complicated than I would have liked, but I think it is correct.
I need to review the test file when I have more time, since ensuring this is tested correctly is important
2 participants
Be able to stop reconnection loop (for example when your application is in the
terminationphase).